web/src/app/(bare)/error.tsx (1)

12-13: Consider utilizing the reset prop.

The reset prop is accepted but not utilized in the component. This prop is typically used in Next.js error boundaries to allow users to attempt recovery from errors.

Consider adding a retry button:

-  return <UnreadyBanner error={error} />;
+  return (
+    <div>
+      <UnreadyBanner error={error} />
+      <button onClick={reset}>Retry</button>
+    </div>
+  );

web/src/lib/auth/oauth.ts (3)

1-3: Consider using native Array.filter instead of lodash/fp

While lodash/fp's filter is good for functional programming, the native Array.filter would be simpler and reduce dependencies for this straightforward use case.

-import { filter } from "lodash/fp";
-
 import { AuthProvider } from "@/api/openapi-schema";

---

9-10: Simplify implementation using native Array.filter

If removing lodash/fp dependency, here's a cleaner implementation using native Array.filter:

-export const filterWithLink = (list: AuthProvider[]): OAuthProvider[] =>
-  filter(hasLink)(list);
+export const filterWithLink = (list: AuthProvider[]): OAuthProvider[] =>
+  list.filter(hasLink);

---

12-12: Enhance type safety for OAuth providers

Consider extending the type definition to include provider-specific information and ensure type safety across different OAuth providers.

-export type OAuthProvider = AuthProvider & { link: string };
+export type OAuthProviderName = 'discord' | 'github';
+export type OAuthProvider = AuthProvider & {
+  link: string;
+  name: OAuthProviderName;
+  scope?: string[];
+};

web/src/app/(bare)/auth/[provider]/callback/layout.tsx (2)

6-6: Consider removing the async keyword

The function is marked as async but doesn't contain any await operations. Since this is a layout component that only renders UI, the async keyword appears unnecessary.

-export default async function Layout({ children }: PropsWithChildren) {
+export default function Layout({ children }: PropsWithChildren) {

---

11-19: Consider enhancing navigation accessibility

The navigation links implementation looks good, but consider wrapping them in a nav element with an appropriate ARIA label for better accessibility.

-      <HStack>
+      <HStack as="nav" aria-label="Authentication options">
         <LinkButton size="xs" variant="ghost" href="/password-reset">
           Forgot password
         </LinkButton>

         <LinkButton size="xs" variant="subtle" href="/register">
           Register
         </LinkButton>
       </HStack>

app/resources/account/authentication/service.go (1)

14-16: Document migration path for existing LinkedIn users

While removing the LinkedIn provider aligns with the PR objectives due to low usage, we should ensure there's a clear migration path for any existing users who might have linked their accounts through LinkedIn.

Consider:

1. Adding documentation about handling existing LinkedIn-linked accounts
2. Implementing a notification system to inform affected users
3. Providing steps for users to link their accounts using alternative providers

web/src/lib/auth/utils.ts (1)

Line range hint 14-24: Consider enhancing provider type safety with discriminated unions

Given the PR's goal of reworking OAuth providers and consolidating account linking, consider enhancing type safety by:

1. Using a discriminated union type for different provider types (password, phone, webauthn, oauth)
2. Adding runtime type guards for provider validation

This would make future provider additions/removals more maintainable and type-safe.

Example type enhancement:

type BaseProvider = {
  provider: string;
};

type OAuthProvider = BaseProvider & {
  kind: 'oauth';
  link: string;
};

type PasswordProvider = BaseProvider & {
  kind: 'password';
};

type AuthProvider = OAuthProvider | PasswordProvider | ...;

web/src/components/settings/AuthMethodSettings/useAuthMethodSettings.ts (1)

1-1: Consider using native Array methods instead of lodash

The filtering logic could be simplified by using native Array methods instead of lodash's find:

-import { find } from "lodash";

 // Remove any OAuth providers that are already active
-const availableOAuth = oauth.filter((v) => {
-  return !find(methods, (m) => m.provider.provider === v.provider);
-});
+const availableOAuth = oauth.filter((v) => 
+  !methods.some((m) => m.provider.provider === v.provider)
+);

This change would:

• Remove an external dependency
• Make the code more concise
• Use more idiomatic JavaScript

Also applies to: 28-30

web/src/api/server.ts (1)

Line range hint 34-37: Consider improving type safety for Orval integration.

The type casting workaround (as T) for Orval's incorrect type generation could mask runtime errors, especially critical for OAuth2 flows.

Consider:

1. Creating a type guard to validate the response shape
2. Adding runtime checks before the cast
3. Opening an issue with Orval to fix the type generation

web/src/components/auth/OAuthProviderList.tsx (2)

16-18: Address TODO and improve code comments

The comment style could be more professional, and the TODO indicates missing functionality.

Would you like me to help create a GitHub issue to track the admin configuration feature implementation?

---

45-51: Consider accessibility and type safety improvements

The link button implementation could be enhanced with:

1. ARIA labels for better screen reader support
2. Visual indication of the OAuth provider (e.g., icons)

 function OAuthProviderLink({ provider }: { provider: OAuthProvider }) {
   return (
     <LinkButton
       size="sm"
       variant="outline"
       w="full"
       href={provider.link}
+      aria-label={`Sign in with ${provider.name}`}
     >
+      {provider.icon && <Icon name={provider.icon} aria-hidden="true" />}
       {provider.name}
     </LinkButton>
   );
 }

web/src/components/settings/AuthMethodSettings/OAuth/OAuth.tsx (1)

30-51: Consider improving identifier display format

The active accounts section has been greatly improved with better visual hierarchy and additional context. However, the identifier display could be enhanced.

Consider this improvement for the identifier display:

-  <styled.pre fontSize="sm">id:{v.identifier}</styled.pre>
+  <styled.code fontSize="sm" color="fg.subtle">ID: {v.identifier}</styled.code>

app/services/authentication/service.go (2)

70-70: LGTM: Provider registration looks good

The Discord provider is correctly added to the providers slice. The debug logging will automatically include it via the existing mapping functions.

Consider adding a test case to verify the Discord provider is correctly included in the provider list:

func TestDiscordProviderRegistration(t *testing.T) {
    m := New(
        zap.NewNop(),
        mockSettings,
        nil, nil, nil, nil, nil,
        &discord.Provider{}, nil,
    )
    
    provider, err := m.Provider(authentication.ServiceOAuthDiscord)
    assert.NoError(t, err)
    assert.NotNil(t, provider)
}

---

Line range hint 1-108: Architecture maintains clean separation of concerns

The changes preserve the existing architecture's strengths:

• Clean provider interface abstraction
• Consistent dependency injection pattern
• Extensible provider registration system

This makes future provider additions/removals straightforward and maintainable.

Consider documenting the provider addition/removal process in the README to help future contributors:

1. Add/remove provider package
2. Update imports
3. Modify Build() registration
4. Update New() parameters
5. Add/remove from providers slice

web/src/api/common.ts (1)

80-82: LGTM! Consider adding documentation.

The addition of string type handling is a good improvement, especially in the context of OAuth2 provider integration. This prevents double stringification of data that's already in string format.

Consider adding a comment to document this case:

  if (data instanceof File || data instanceof Blob) {
    return data;
  }

+  // Handle pre-formatted string payloads (e.g., OAuth2 token requests)
  if (typeof data === "string") {
    return data;
  }

app/services/authentication/provider/oauth/google/google.go (2)

40-42: Add field documentation

Consider adding documentation comments for the struct fields to improve code maintainability.

 type Provider struct {
+	// register handles account registration and lookup operations
 	register *register.Registrar
+	// ed handles encryption/decryption of OAuth state
 	ed       endec.EncrypterDecrypter
 
+	// callback is the OAuth redirect URL
 	callback string
+	// config contains OAuth provider configuration
 	config   *all.Configuration
 }

---

Line range hint 74-89: Consider explicit error handling for missing configuration

The nil check silently returns nil when config is missing. Consider making this more explicit to avoid potential runtime issues.

-func (p *Provider) oauthConfig() *oauth2.Config {
+func (p *Provider) oauthConfig() (*oauth2.Config, error) {
 	if p.config == nil {
-		return nil
+		return nil, fault.New("oauth configuration is not initialized")
 	}
 
 	return &oauth2.Config{
		ClientID:     p.config.ClientID,
		ClientSecret: p.config.ClientSecret,
		Endpoint:     google.Endpoint,
		RedirectURL:  p.callback,
		Scopes: []string{
			"https://www.googleapis.com/auth/userinfo.email",
			"https://www.googleapis.com/auth/userinfo.profile",
		},
-	}
+	}, nil
 }

web/src/app/(bare)/auth/[provider]/callback/page.tsx (2)

21-21: Remove unused props parameter and update component definition

After replacing props.params and props.searchParams with the hooks useParams and useSearchParams, the props parameter is no longer needed. You can remove it from the component definition.

Apply this diff:

-export default function Page(props: Props) {
+export default function Page() {
   const router = useRouter();

---

3-4: Remove unused use import from React

Since you're no longer using the use function from React, you can remove it from the import statement to clean up the code.

Apply this diff:

 import { useRouter } from "next/navigation";
-import { use, useEffect, useState } from "react";
+import { useEffect, useState } from "react";

app/services/authentication/provider/oauth/discord/discord.go (1)

129-130: Fallback to Username if GlobalName is empty

u.GlobalName may be empty for some users. Consider falling back to u.Username to ensure the name variable is always populated.

Suggested change:

     handle := strings.ToLower(u.Username)
-    name := u.GlobalName
+    name := u.GlobalName
+    if name == "" {
+        name = u.Username
+    }

app/services/account/register/register.go (3)

Line range hint 74-74: Fix comparison of value to pointer in onboarding status check

In the Create method, the comparison if status == &onboarding.StatusRequiresFirstAccount is incorrect because it compares a value to the address of a value. You should compare the value directly without taking its address.

Here's the diff to fix the comparison:

-if status == &onboarding.StatusRequiresFirstAccount {
+if status == onboarding.StatusRequiresFirstAccount {

---

85-216: Consider refactoring to reduce code duplication

The methods GetOrCreateViaEmail and GetOrCreateViaHandle have similar structures and logic, handling account creation and linking based on different identifiers. Consider refactoring common code into shared helper functions to improve maintainability and reduce duplication.

---

Line range hint 74-78: Ensure test coverage for first-time setup scenario

The condition handling for first-time setup in the Create method is crucial for setting the first account to admin. Ensure that there are unit tests covering this scenario to verify that the onboarding status is correctly interpreted and that the admin flag is properly set.

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Check how the error component is integrated in the OAuth callback flow

# Find OAuth callback pages that might use this error component
rg -l "error.tsx|ErrorPage" "web/src/app/(bare)/auth/"

# Check for error handling patterns in OAuth callback
ast-grep --pattern 'catch (error) { $$$}' web/src/app/(bare)/auth/

#!/bin/bash
# Let's try a different approach to verify error handling in OAuth flow

# Find OAuth-related files in the auth directory
fd . web/src/app/\(bare\)/auth/ -t f

# Look for error handling patterns in these files
rg -n "catch|throw|error" web/src/app/\(bare\)/auth/

# Check if UnreadyBanner is used elsewhere
rg -n "UnreadyBanner" web/src/

#!/bin/bash
# Search for provider configurations and ensure LinkedIn is removed and Discord/GitHub are properly configured
echo "Checking for OAuth provider configurations..."
rg -l "linkedin|discord|github" --type ts

# Check for any remaining LinkedIn OAuth references
echo "Checking for remaining LinkedIn references..."
rg "linkedin" --type ts

# Verify Discord and GitHub provider implementations
echo "Verifying Discord and GitHub implementations..."
rg "discord|github" --type ts

#!/bin/bash
# Check for OAuth provider configurations in API schema and related files
echo "Checking API schema for provider definitions..."
fd "openapi-schema" --exec rg -A 5 "AuthProvider|OAuthProvider" {}

# Look for provider-specific configuration or setup
echo "Checking for provider setup code..."
rg -A 5 "provider.*=|provider.*type" --type ts

# Search for any OAuth-related configuration files
echo "Looking for OAuth configuration files..."
fd -e ts -e json --exec rg -l "oauth|auth.*provider" {}

#!/bin/bash
# Check the provider implementation files
echo "Checking provider implementations..."
cat web/src/lib/auth/providers.ts

# Check auth utility functions
echo "Checking auth utilities..."
cat web/src/lib/auth/utils.ts

# Check OpenAPI client implementation for auth
echo "Checking OpenAPI auth client..."
cat web/src/api/openapi-client/auth.ts

#!/bin/bash
# Search for OAuth provider configuration in server code
echo "Checking server-side OAuth configuration..."
fd -e go --exec rg -A 5 "oauth.*provider|auth.*provider" {}

# Look for provider type definitions
echo "Checking provider type definitions..."
fd -e go --exec rg -A 3 "type.*Provider" {}

#!/bin/bash
# Description: Check the implementation of OAuthProviderList to verify prop requirements

# Search for the OAuthProviderList component definition
ast-grep --pattern 'export function OAuthProviderList($_) {
  $$$
}'

# Alternative: Look for type/interface definitions
ast-grep --pattern 'type OAuthProviderListProps = {
  $$$
}'

# Alternative: Look for interface definitions
ast-grep --pattern 'interface OAuthProviderListProps {
  $$$
}'

#!/bin/bash
# Let's try a broader search to locate the OAuthProviderList component
rg -l "OAuthProviderList" --type ts --type tsx

# Then let's look at the content of any files that contain this component
rg "OAuthProviderList" -A 10 --type ts --type tsx

#!/bin/bash
# Search for files containing OAuthProviderList without file type restrictions
fd . -e tsx -e ts | xargs rg "OAuthProviderList" -A 10

#!/bin/bash
# Description: Search for any remaining LinkedIn-related code that needs cleanup

echo "Searching for LinkedIn-related code..."
rg -i "linkedin" --type go --type typescript --type javascript

echo "Searching for specific OAuth LinkedIn constants..."
rg -i "oauth_linkedin|serviceOAuthLinkedin" --type go --type typescript --type javascript

#!/bin/bash
# Let's try a more focused search without type restrictions
echo "Searching for LinkedIn-related code..."
rg -i "linkedin"

echo "Searching for specific OAuth LinkedIn constants..."
rg -i "oauth_linkedin|serviceOAuthLinkedin"

# Let's also check for any configuration or migration files
echo "Checking configuration files..."
fd -e yaml -e json -e toml -e conf | xargs rg -i "linkedin"

#!/bin/bash
# Description: Verify the filterWithLink implementation and its type safety

# Check the implementation of filterWithLink
ast-grep --pattern 'export function filterWithLink($_) {
  $$$
}'

# Check for OAuth provider type definitions
rg -A 5 'type.*Provider.*=' 

#!/bin/bash
# Search for filterWithLink implementation
ast-grep --pattern 'function filterWithLink($_) {
  $$$
}'

# Also search for any usage of filterWithLink
rg 'filterWithLink' -A 3

#!/bin/bash
# Search for hasLink implementation and related type definitions
ast-grep --pattern 'const hasLink = $_'

# Search for AuthProvider type definition
ast-grep --pattern 'type AuthProvider = $_'

# Also search for any usage of hasLink
rg 'hasLink' -A 3

#!/bin/bash
# Description: Check for OAuth2-related header usage across the codebase
# Look for Authorization, Bearer, OAuth headers to ensure compatibility

rg -i 'authorization|bearer|oauth' --type ts

#!/bin/bash
# Let's check the actual header usage in the server.ts file and related API files
rg -A 5 'headers' web/src/api/

#!/bin/bash
# Search for any remaining usage of AuthProvider type in props or OAuth context
rg -t typescript "AuthProvider\[\]" 
# Search for components consuming the OAuth component
ast-grep --pattern 'import { OAuth } from "@/components/settings/AuthMethodSettings/OAuth"'

#!/bin/bash
# Search for AuthProvider type usage with correct file extension
rg -t ts -t tsx "AuthProvider\[\]"

# Search for OAuth component imports with correct extension
rg -t ts -t tsx 'import.*OAuth.*from.*AuthMethodSettings'

# Search for OAuthProvider type usage
rg -t ts -t tsx "OAuthProvider\[\]"

# Look for type definitions of AuthProvider and OAuthProvider
rg -t ts -t tsx "type (Auth|OAuth)Provider"

#!/bin/bash
# Search for files with TypeScript extensions first
fd -e ts -e tsx

# Then use these file patterns with ripgrep
rg "AuthProvider\[\]" $(fd -e ts -e tsx)

# Search for OAuth component imports
rg 'import.*OAuth.*from.*AuthMethodSettings' $(fd -e ts -e tsx)

# Search for OAuthProvider type usage
rg "OAuthProvider\[\]" $(fd -e ts -e tsx)

# Look for type definitions
rg "type (Auth|OAuth)Provider" $(fd -e ts -e tsx)

#!/bin/bash
# Search for any remaining references to LinkedIn provider parameters
rg -A 3 '\*linkedin\.Provider' --type go

# Search for any missed Discord provider parameter references
rg -A 3 '\*discord\.Provider' --type go

#!/bin/bash
# Search for any references to LinkedIn in the codebase
rg -i "linkedin" --type go

# Search for any references to Discord in function calls or method invocations
ast-grep --pattern 'dp.$$$'

#!/bin/bash
# Search for function signatures and their usage in authentication service
rg -A 5 "func.*Manager" --type go

# Search for provider initialization or usage patterns
rg -B 2 -A 5 "Provider.*{" --type go

# Look for any authentication-related function calls
rg -B 2 "authenticate|login|signin" --type go -i

#!/bin/bash
# Search for any remaining LinkedIn OAuth-related imports
rg -l 'linkedin' --type go

#!/bin/bash
# Search for any remaining LinkedIn-related code with broader patterns
rg -i "linkedin" --type go

# Also check for any OAuth provider imports that might contain LinkedIn
ast-grep --pattern 'import (
  $$$
  "github.com/Southclaws/storyden/app/services/authentication/provider/oauth/$_"
  $$$
)'

#!/bin/bash
# Search for any remaining LinkedIn-related code in the authentication service
rg -i "linkedin" app/services/authentication/

# Check for any LinkedIn provider-related code
ast-grep --pattern 'type Provider struct {
  $$$
  linkedin $$$
  $$$
}'

#!/bin/bash
# Search for buildRequest usage in OAuth2-related files
echo "Checking buildRequest usage in OAuth2 context..."
rg -A 5 "buildRequest.*data:" --type ts

# Look for potential OAuth2 token request handling
echo "Checking OAuth2 token-related code..."
rg -l "oauth|token|auth" --type ts

#!/bin/bash
# Search for OAuth2-related code implementation
echo "Checking OAuth2 implementation..."
ast-grep --pattern 'buildRequest($$$)' 

# Search for string data handling in auth-related files
echo "Checking string data handling in auth files..."
rg "data.*string" web/src/api/ web/src/lib/auth/ -A 5

-	email, err := mail.ParseAddress(u.Email)
-	if err != nil {
-		return nil, fault.Wrap(err, fctx.With(ctx))
+	email, err := mail.ParseAddress(u.Email)
+	if err != nil {
+		return nil, fault.Wrap(err,
+			fctx.With(ctx),
+			ftag.With(ftag.InvalidArgument),
+			fmsg.WithDesc("invalid email format", "The email address from Google is not valid."))
 	}

#!/bin/bash
# Check for potential handle collisions in existing codebase
rg -g '!*.test.*' -g '!*_test.go' "petname.Generate.*" -A 2

# Check for handle uniqueness validation in the register package
rg -g '*.go' "func.*GetOrCreateViaEmail" ./app/services/account/register -A 10

#!/bin/bash
# Check handle uniqueness validation in account writer
ast-grep --pattern 'func $_ Create($_, $handle string, $$$) {
  $$$
}'

# Check for any handle validation or collision detection
rg -g '*.go' "handle.*exists|duplicate.*handle|unique.*handle" -B 2 -A 2

#!/bin/bash
# Description: Check for any remaining LinkedIn OAuth references
# that might need cleanup

rg -i "linkedin|ServiceOAuthLinkedin" --type go

#!/bin/bash
# Let's do a broader search to verify OAuth-related changes
# First, let's check for any OAuth-related service definitions
ast-grep --pattern 'ServiceOAuth$_ = Service{$_}'

# Then, let's check OAuth-related constants and configurations
rg -i "oauth.*linkedin" 

# Finally, let's check for any OAuth provider configurations
rg -i "provider.*linkedin"

-return s.CreateWithHandle(ctx, service, authName, identifier, token, name, handle)
+return s.CreateWithHandle(ctx, service, authName, identifier, token, name, handle)

web/src/app/(bare)/auth/[provider]/callback/page.tsx (2)

Line range hint 13-20: Consider simplifying Props type definition

The Props type uses Promises for params and searchParams, which is unusual for React component props. This could lead to unnecessary complexity in handling the async values.

Consider using direct types instead:

export type Props = {
-  params: Promise<{
+  params: {
    provider: string;
-  }>;
+  };
-  searchParams: Promise<OAuthCallback>;
+  searchParams: OAuthCallback;
};

---

46-51: Enhance error handling

The current error handling could be more informative for users.

Consider adding error type checking and specific error messages:

      {
        errorToast: false,
        onError: async (e) => {
-         setError(e);
+         if (e instanceof Response) {
+           const data = await e.json();
+           setError(`Authentication failed: ${data.message}`);
+         } else {
+           setError(`Authentication failed: ${e}`);
+         }
        },
      },

Configuration used: CodeRabbit UI
Review profile: CHILL